Skip to content

feat(wanda): Add 'params' field for env var validation and discovery#368

Draft
andrew-anyscale wants to merge 1 commit intomainfrom
andrew/revup/main/wanda-params
Draft

feat(wanda): Add 'params' field for env var validation and discovery#368
andrew-anyscale wants to merge 1 commit intomainfrom
andrew/revup/main/wanda-params

Conversation

@andrew-anyscale
Copy link
Contributor

@andrew-anyscale andrew-anyscale commented Jan 13, 2026

Add params field to wanda spec for declaring allowed values of environment variables used in templated fields (name, froms). This enables:

  • Strict validation: reject builds where env var values don't match
    declared params
  • Dependency discovery: specs with params are indexed under all their
    expanded names (e.g., base$PY with params [3.10, 3.11] is indexed as
    both base3.10 and base3.11), enabling dependency resolution without
    requiring all env vars to be set

Validation runs before variable expansion in buildDepGraph, so errors
reference the original $VARNAME.

  name: rayml$PY_VERSION
  params:
    PY_VERSION:
      - "3.10"
      - "3.11"
      - "3.12"
  froms:
    - cr.ray.io/rayproject/base$PY_VERSION
  dockerfile: rayml.Dockerfile
  # Valid - succeeds
  PY_VERSION=3.11 wanda rayml.wanda.yaml

  # Invalid - fails with validation error
  PY_VERSION=3.9 wanda rayml.wanda.yaml
  # Error: env var PY_VERSION has value "3.9", but params only allow: [3.10 3.11 3.12]

Topic: wanda-params
Relative: wanda-deps

Signed-off-by: andrew andrew@anyscale.com

@andrew-anyscale
Copy link
Contributor Author

andrew-anyscale commented Jan 13, 2026

Reviews in this chain:
#368 feat(wanda): Add 'params' field for env var validation and discovery

@andrew-anyscale
Copy link
Contributor Author

andrew-anyscale commented Jan 13, 2026

# head base diff date summary
0 f8311352 388c9c17 diff Jan 12 17:04 PM 4 files changed, 553 insertions(+), 5 deletions(-)
1 0337224c 05c0ded9 diff Jan 12 17:12 PM 2 files changed, 58 insertions(+), 33 deletions(-)
2 cd893e4c 839b0867 diff Jan 12 17:22 PM 1 file changed, 1 insertion(+), 1 deletion(-)
3 aa778408 839b0867 diff Jan 12 17:29 PM 2 files changed, 33 insertions(+), 42 deletions(-)
4 c1be0808 308598c1 rebase Jan 12 17:41 PM 0 files changed
5 982cc460 e8c1f1b2 rebase Jan 12 17:49 PM 0 files changed
6 e1c5fedc a8a69082 rebase Jan 12 18:07 PM 0 files changed
7 aa3f891a 4d7b3449 rebase Jan 12 18:15 PM 0 files changed
8 590a1f98 b8380304 rebase Jan 12 18:24 PM 0 files changed
9 a0cda8c9 a289e514 rebase Jan 12 18:25 PM 0 files changed
10 91301758 2327511e rebase Jan 12 18:37 PM 0 files changed
11 7367abfc fdbd39ef rebase Jan 12 18:49 PM 0 files changed
12 0c9f9450 86b1c2e1 rebase Jan 12 18:57 PM 0 files changed
13 6539ed83 00b3ca37 rebase Jan 12 19:22 PM 0 files changed
14 bbd00709 00b3ca37 diff Jan 12 19:30 PM 1 file changed, 3 insertions(+), 3 deletions(-)
15 3a439d33 1a7af24d diff Jan 13 8:30 AM 2 files changed, 24 insertions(+), 66 deletions(-)
16 44d91e73 b0597a44 rebase Jan 13 8:45 AM 0 files changed
17 59be6945 c0d07aea rebase Jan 13 8:49 AM 0 files changed
18 922cdab9 e20bf2d1 rebase Jan 13 9:32 AM 0 files changed
19 32e8cb9d 7c9b96c9 diff Jan 13 11:43 AM 1 file changed, 6 insertions(+), 4 deletions(-)
20 5ca62ee2 cd5a547a diff Jan 13 11:51 AM 1 file changed, 4 insertions(+), 4 deletions(-)
21 56081ddb 987fd295 rebase Jan 13 11:56 AM 0 files changed
22 c63731b9 543dfc49 diff Jan 13 16:28 PM 1 file changed, 8 insertions(+), 9 deletions(-)
23 80ff2eae 5a582a1d rebase Jan 13 16:30 PM 0 files changed
24 fafb9d9f 35530d6d rebase Jan 13 16:48 PM 0 files changed
25 b4efcec4 a384fdf6 rebase Jan 13 17:23 PM 0 files changed
26 54087176 df64f501 diff Jan 13 21:39 PM 0 files changed

@gemini-code-assist
Copy link
Contributor

Warning

You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again!

@andrew-anyscale andrew-anyscale changed the base branch from andrew/revup/main/wanda-deps to andrew/revup/main/wanda-build-deps January 13, 2026 01:12
@andrew-anyscale andrew-anyscale force-pushed the andrew/revup/main/wanda-params branch 2 times, most recently from 0337224 to cd893e4 Compare January 13, 2026 01:22
@andrew-anyscale andrew-anyscale force-pushed the andrew/revup/main/wanda-build-deps branch from 05c0ded to 839b086 Compare January 13, 2026 01:23
@andrew-anyscale andrew-anyscale force-pushed the andrew/revup/main/wanda-params branch from cd893e4 to aa77840 Compare January 13, 2026 01:29
@andrew-anyscale andrew-anyscale changed the title feat(wanda): Add params field for env var validation and discovery feat(wanda): Add 'params' field for env var validation and discovery Jan 13, 2026
@andrew-anyscale andrew-anyscale force-pushed the andrew/revup/main/wanda-params branch from aa77840 to c1be080 Compare January 13, 2026 01:41
@andrew-anyscale andrew-anyscale force-pushed the andrew/revup/main/wanda-build-deps branch from 839b086 to 308598c Compare January 13, 2026 01:41
@andrew-anyscale andrew-anyscale force-pushed the andrew/revup/main/wanda-params branch from c1be080 to 982cc46 Compare January 13, 2026 01:49
@andrew-anyscale andrew-anyscale force-pushed the andrew/revup/main/wanda-build-deps branch from 308598c to e8c1f1b Compare January 13, 2026 01:49
@andrew-anyscale andrew-anyscale force-pushed the andrew/revup/main/wanda-params branch from 982cc46 to e1c5fed Compare January 13, 2026 02:07
@andrew-anyscale andrew-anyscale force-pushed the andrew/revup/main/wanda-build-deps branch from e8c1f1b to a8a6908 Compare January 13, 2026 02:07
@andrew-anyscale andrew-anyscale force-pushed the andrew/revup/main/wanda-params branch from e1c5fed to aa3f891 Compare January 13, 2026 02:15
@andrew-anyscale andrew-anyscale force-pushed the andrew/revup/main/wanda-build-deps branch 2 times, most recently from 4d7b344 to b838030 Compare January 13, 2026 02:24
@andrew-anyscale andrew-anyscale force-pushed the andrew/revup/main/wanda-params branch 2 times, most recently from 590a1f9 to a0cda8c Compare January 13, 2026 02:25
@andrew-anyscale andrew-anyscale force-pushed the andrew/revup/main/wanda-build-deps branch 2 times, most recently from a289e51 to 2327511 Compare January 13, 2026 02:37
@andrew-anyscale andrew-anyscale force-pushed the andrew/revup/main/wanda-params branch from a0cda8c to 9130175 Compare January 13, 2026 02:37
@andrew-anyscale andrew-anyscale force-pushed the andrew/revup/main/wanda-build-deps branch from 2327511 to fdbd39e Compare January 13, 2026 02:49
@andrew-anyscale andrew-anyscale force-pushed the andrew/revup/main/wanda-params branch 2 times, most recently from 7367abf to 0c9f945 Compare January 13, 2026 02:57
@andrew-anyscale andrew-anyscale force-pushed the andrew/revup/main/wanda-build-deps branch from fdbd39e to 86b1c2e Compare January 13, 2026 02:57
@andrew-anyscale andrew-anyscale force-pushed the andrew/revup/main/wanda-params branch 2 times, most recently from bbd0070 to 3a439d3 Compare January 13, 2026 16:30
@andrew-anyscale andrew-anyscale force-pushed the andrew/revup/main/wanda-build-deps branch from 00b3ca3 to 1a7af24 Compare January 13, 2026 16:30
@andrew-anyscale andrew-anyscale force-pushed the andrew/revup/main/wanda-params branch from 3a439d3 to 44d91e7 Compare January 13, 2026 16:45
@andrew-anyscale andrew-anyscale force-pushed the andrew/revup/main/wanda-build-deps branch 2 times, most recently from b0597a4 to c0d07ae Compare January 13, 2026 16:49
@andrew-anyscale andrew-anyscale force-pushed the andrew/revup/main/wanda-params branch 2 times, most recently from 59be694 to 922cdab Compare January 13, 2026 17:32
@andrew-anyscale andrew-anyscale force-pushed the andrew/revup/main/wanda-build-deps branch 2 times, most recently from e20bf2d to 7c9b96c Compare January 13, 2026 19:43
@andrew-anyscale andrew-anyscale force-pushed the andrew/revup/main/wanda-params branch from 922cdab to 32e8cb9 Compare January 13, 2026 19:43
@andrew-anyscale andrew-anyscale force-pushed the andrew/revup/main/wanda-build-deps branch from 7c9b96c to cd5a547 Compare January 13, 2026 19:51
@andrew-anyscale andrew-anyscale force-pushed the andrew/revup/main/wanda-params branch 2 times, most recently from 5ca62ee to 56081dd Compare January 13, 2026 19:56
@andrew-anyscale andrew-anyscale force-pushed the andrew/revup/main/wanda-build-deps branch from cd5a547 to 987fd29 Compare January 13, 2026 19:56
@andrew-anyscale andrew-anyscale force-pushed the andrew/revup/main/wanda-params branch from 56081dd to c63731b Compare January 14, 2026 00:28
@andrew-anyscale andrew-anyscale force-pushed the andrew/revup/main/wanda-build-deps branch 2 times, most recently from 543dfc4 to 5a582a1 Compare January 14, 2026 00:30
@andrew-anyscale andrew-anyscale force-pushed the andrew/revup/main/wanda-params branch 2 times, most recently from 80ff2ea to fafb9d9 Compare January 14, 2026 00:48
@andrew-anyscale andrew-anyscale force-pushed the andrew/revup/main/wanda-build-deps branch 2 times, most recently from 35530d6 to a384fdf Compare January 14, 2026 01:23
@andrew-anyscale andrew-anyscale force-pushed the andrew/revup/main/wanda-params branch from fafb9d9 to b4efcec Compare January 14, 2026 01:23
Base automatically changed from andrew/revup/main/wanda-build-deps to main January 14, 2026 05:37
Add `params` field to wanda spec for declaring allowed values of
environment variables used in templated fields (name, froms). This
enables:
- Strict validation: reject builds where env var values don't match
  declared params
- Dependency discovery: specs with params are indexed under all their
  expanded names (e.g., base$PY with params [3.10, 3.11] is indexed as
  both base3.10 and base3.11), enabling dependency resolution without
  requiring all env vars to be set

Validation runs before variable expansion in buildDepGraph, so errors
reference the original $VARNAME.

Topic: wanda-params
Relative: wanda-build-deps

Signed-off-by: andrew <andrew@anyscale.com>
@andrew-anyscale andrew-anyscale force-pushed the andrew/revup/main/wanda-params branch from b4efcec to 5408717 Compare January 14, 2026 05:39
@gitar-bot
Copy link

gitar-bot bot commented Jan 14, 2026

Code Review 👍 Approved with suggestions 0 resolved / 3 findings

Well-designed feature with comprehensive test coverage. A few minor robustness improvements suggested around edge cases.

Suggestions 💡 3 suggestions
Edge Case: extractVarNames parsing differs from test expectations

📄 wanda/spec.go:209

Looking at the test case "prefix$VAR1-$VAR2suffix" which expects ["VAR1", "VAR2suffix"], this suggests that variables without explicit termination (like - after VAR1) correctly stop at non-alphanumeric characters, but VAR2suffix shows that suffix is being included as part of the variable name.

This is correct behavior per the parsing logic (letters/underscores continue the variable name), but the test expectation reveals a potential usability concern: users might expect $VAR2suffix to expand $VAR2 only and leave suffix as literal text. This is an edge case worth documenting, though the behavior is internally consistent with how the parser works.

Edge Case: Potential exponential blowup in cartesian product generation

📄 wanda/spec.go:210

The expandVarWithParams function generates the cartesian product of all param combinations. With multiple variables each having many values, this could produce a very large number of combinations.

For example, with 3 variables each having 10 values, this would generate 1000 combinations. While this is likely acceptable for typical use cases (2-3 Python versions, 2 architectures), there's no upper bound or warning for unusual configurations.

Consider adding a safeguard or warning if the number of combinations exceeds a reasonable threshold (e.g., 100), to help users catch misconfiguration early:

const maxCombinations = 100
if len(combinations) > maxCombinations {
    return nil, fmt.Errorf("params generate %d combinations, exceeding limit of %d", len(combinations), maxCombinations)
}
Code Quality: Map iteration order in ValidateParams is non-deterministic

📄 wanda/spec.go:211 📄 wanda/spec.go:156

In ValidateParams, the function iterates over s.Params map. Since Go map iteration order is non-deterministic, if multiple params have invalid values, the error message will vary between runs, potentially reporting different variables each time.

While this doesn't affect correctness (any validation failure is still a failure), it could make debugging harder since users might see different error messages for the same configuration.

Consider collecting all validation errors or sorting the keys before iteration:

func (s *Spec) ValidateParams(lookup lookupFunc) error {
    var errs []string
    for varName, allowed := range s.Params {
        val, ok := lookup(varName)
        if !ok {
            continue
        }
        valid := false
        for _, a := range allowed {
            if val == a {
                valid = true
                break
            }
        }
        if !valid {
            errs = append(errs, fmt.Sprintf("%s=%q (allowed: %v)", varName, val, allowed))
        }
    }
    if len(errs) > 0 {
        sort.Strings(errs)
        return fmt.Errorf("invalid param values: %s", strings.Join(errs, ", "))
    }
    return nil
}

What Works Well

Clean implementation of the params feature with clear separation between validation and expansion. Thorough test coverage including edge cases like cartesian products, deduplication, and env var fallback. Excellent error messages that include valid values to guide users.

Options

Auto-apply is off Gitar will not commit updates to this branch.
Display: compact Hiding non-applicable rules.

Comment with these commands to change:

Auto-apply Compact
gitar auto-apply:on         
gitar display:verbose         

Was this helpful? React with 👍 / 👎 | This comment will update automatically (Docs)

Copy link
Collaborator

@aslonnie aslonnie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe chat, but how does it handle free-formed string type env var expansion now?

@andrew-anyscale andrew-anyscale marked this pull request as draft February 2, 2026 16:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants